Conversation
Moves version stamping from read-time injection in getFormDefinition to write-time stamping inside modifyDraft and insertDraft. Every draft mutation now atomically allocates a version, stamps the definition, and snapshots to form-versions. Service callers no longer need to call createFormVersion after modifyDraft. getFormDefinition reverts to a pure read. createLiveFromDraft and createDraftFromLive inherit the stamp on copy. Fixes the submission crash where live reads on forms with unsaved drafts were tagged with draft-save version numbers.
There was a problem hiding this comment.
Pull request overview
This PR refactors form version stamping so $$__formVersion is persisted at write-time (via modifyDraft/insertDraft and related flows) rather than injected at read-time in getFormDefinition, making reads pure and ensuring version snapshots are created atomically during draft mutations.
Changes:
- Move
$$__formVersionstamping + version snapshot creation into repository-layer draft writes (insertDraft/modifyDraft), and makegetFormDefinitiona pure read. - Update multiple draft-mutation services to stop calling
createFormVersionexplicitly (versioning now happens inside draft writes). - Add/adjust supporting APIs and tests (e.g.,
setFormVersion, route stamping, reinstate flow, and versioning tests).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/forms.js | Switch version-definition endpoint stamping from removed injectFormVersion to stampFormVersion. |
| src/helpers/feedback-form/reinstate.test.js | Mock version creation to accommodate new reinstate/version behavior. |
| src/helpers/feedback-form/reinstate.js | Mirror version stamp onto Live during feedback-form reinstate. |
| src/api/forms/service/versioning.test.js | Update expectations to reflect persisted stamp and repository setFormVersion call. |
| src/api/forms/service/versioning.js | Refactor version creation to use shared allocation + stamping helpers and persist the stamp. |
| src/api/forms/service/sections.test.js | Remove expectation that services explicitly call createFormVersion. |
| src/api/forms/service/sections.js | Stop explicitly creating versions for section assignment (now handled by draft writes). |
| src/api/forms/service/page.js | Stop explicitly creating versions for page mutations (now handled by draft writes). |
| src/api/forms/service/options.js | Stop explicitly creating versions for option mutations (now handled by draft writes). |
| src/api/forms/service/migration.js | Stop explicitly creating versions for draft migration updates (now handled by draft writes). |
| src/api/forms/service/lists.js | Stop explicitly creating versions for list mutations (now handled by draft writes). |
| src/api/forms/service/index.test.js | Remove expectations around explicit createFormVersion calls. |
| src/api/forms/service/index.js | Remove explicit version creation in create/title update paths; keep versioning for metadata-only edits. |
| src/api/forms/service/definition.test.js | Update tests to assert getFormDefinition returns stored definition verbatim (no injection). |
| src/api/forms/service/definition.js | Remove read-time version injection (injectFormVersion) and stop explicit versioning calls. |
| src/api/forms/service/conditions.js | Stop explicitly creating versions for condition mutations (now handled by draft writes). |
| src/api/forms/service/component.js | Stop explicitly creating versions for component mutations (now handled by draft writes). |
| src/api/forms/repositories/helpers.js | Add FORM_VERSION_METADATA_KEY, allocateDraftVersion, stampFormVersion; stamp+snapshot inside insertDraft/modifyDraft. |
| src/api/forms/repositories/form-definition-repository.test.js | Mock metadata/versions repositories to avoid new side effects during repository tests. |
| src/api/forms/repositories/form-definition-repository.js | Add setFormVersion to persist a version stamp onto stored definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
406
to
+410
| const repositioned = repositionPaymentAndSummary(updated) | ||
| const validated = validate(repositioned, schema) | ||
|
|
||
| // Validate form definition | ||
| const draft = validate(repositioned, schema) | ||
| const versionMetadata = await allocateDraftVersion(formId, session) | ||
| const draft = stampFormVersion(validated, versionMetadata) |
Comment on lines
+320
to
+328
| await formVersionsRepository.createVersion( | ||
| { | ||
| formId, | ||
| versionNumber: versionMetadata.versionNumber, | ||
| formDefinition: draft, | ||
| createdAt: versionMetadata.createdAt | ||
| }, | ||
| session | ||
| ) |
Comment on lines
+429
to
+437
| await formVersionsRepository.createVersion( | ||
| { | ||
| formId, | ||
| versionNumber: versionMetadata.versionNumber, | ||
| formDefinition: draft, | ||
| createdAt: versionMetadata.createdAt | ||
| }, | ||
| session | ||
| ) |
…ing tests Addresses Copilot review feedback on PR #794: - modifyDraft's initial findOne now participates in the transaction (was reading outside the session, risking stale reads under concurrent draft updates). Added projection { draft: 1 } since that's all we need. - Added two tests covering the version stamping behaviour introduced in modifyDraft and insertDraft (allocates version, stamps draft with $$__formVersion, snapshots to form-versions).
|
alexluckett
approved these changes
Apr 24, 2026
Contributor
alexluckett
left a comment
alexluckett
left a comment
There was a problem hiding this comment.
LGTM but let's wait for one more approval before we merge
davidjamesstone
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stamping moves from read-time (
getFormDefinition) to write-time (insidemodifyDraft/insertDraft). Every draft mutation atomically stamps and snapshots;getFormDefinitionis a pure read.Alternative to #792.